Skip to content

Conversation

@Pertempto
Copy link
Contributor

Summary

This PR adds an OpenAPI v3 spec under specs/openapi.yaml and a basic contract test that ensures the spec is loadable and valid YAML. The contract test is intentionally minimal and will be expanded in follow-up PRs to run full OpenAPI linting and server validation.

Related issue: #18

Fixes #18

@github-actions
Copy link
Contributor

The OpenAPI spec is missing several endpoints documented in the API docs, such as /api/v1/acl, /api/v1/user/resetKey, /api/v1/user/generateToken, and /api/v1/user/exchangeToken. Please complete the spec to cover the full public API surface as stated in the PR description and spec file.

@github-actions
Copy link
Contributor

The server URL in the OpenAPI spec is http://localhost:8080/v1, but the API docs use paths starting with /api/v1 (e.g., /api/v1/events). This inconsistency needs to be resolved to match the actual API paths.

@github-actions
Copy link
Contributor

Add proper schemas for the request and response bodies in the OpenAPI spec instead of using vague type: object. Refer to the API docs for details on the event and other object structures.

@github-actions
Copy link
Contributor

Update the test to use os.ReadFile instead of the deprecated ioutil.ReadFile.

@github-actions
Copy link
Contributor

The OpenAPI spec in specs/openapi.yaml appears incomplete. It only defines /health and /events, but the full API documentation includes additional endpoints like /api/v1/acl and /api/v1/user/*. Please update the spec to cover the entire API surface.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 25, 2025

Changes Requested

  • Add a servers block to specs/openapi.yaml defining http://localhost:8080 as the base URL.
  • Add global security with ApiKeyAuth to specs/openapi.yaml, and override with security: [] for unauthenticated endpoints.
  • Change the error response in specs/openapi.yaml for invalid setup tokens in POST /api/v1/user/exchangeToken from 401 to 404 with description "Token not found or expired".
  • Update src/handlers/user.go to return 404 instead of 401 for invalid tokens, adjusting the error message.
  • Update .redocly.lint-ignore.yaml for the updated health endpoint path.

Summary of Changes

  • Added a comprehensive OpenAPI v3 spec for all API endpoints with detailed schemas, examples, error handling, and authentication.
  • Integrated CI workflows for OpenAPI linting with Redocly and server validation with Schemathesis via bash-based contract tests.
  • Updated dependencies for modern Gin and testing libraries.
  • Updated CHANGELOG.md with PR link.
  • Reorganized route grouping in main.go for proper auth middleware application.

Overall Feedback

The OpenAPI spec comprehensively covers the entire /api/v1 API surface with accurate schemas and examples, making it a solid foundation for docs and client SDKs. Switching to bash scripts for contract testing is elegant and lightweight, ensuring the spec stays in sync with the live API during CI. Great job on including security definitions and detailed response models – clients will appreciate the clarity. However, the spec still expects some refinements like explicit global auth (with overrides) and consistent error codes for better tooling support. Keep up the stellar work! 🚀✨

Minor aside: The pattern of * wildcards in ACL could be an example in the spec. No rush in follow-ups!

@github-actions
Copy link
Contributor

There's a mismatch in the API path prefix: the spec uses /v1 (e.g., paths like /health), but the API docs describe endpoints under /api/v1/*. Please clarify and align the server URL and paths.

@github-actions
Copy link
Contributor

The request body schema for POST /events is defined as type: object, but per the API docs, it should accept a JSON array of event objects. Update the schema accordingly.

@github-actions
Copy link
Contributor

In tests/contract/openapi_spec_test.go, you're using the deprecated ioutil.ReadFile. Prefer os.ReadFile for Go 1.16+ as per modern best practices.

@github-actions
Copy link
Contributor

The OpenAPI spec is missing several endpoints documented in the API docs, such as /api/v1/acl, /api/v1/user/resetKey, /api/v1/user/generateToken, and /api/v1/user/exchangeToken. Please complete the spec to cover the full public API surface as stated in the PR description and spec file.

@github-actions
Copy link
Contributor

The server URL in the OpenAPI spec is http://localhost:8080/v1, but the API docs use paths starting with /api/v1 (e.g., /api/v1/events). This inconsistency needs to be resolved to match the actual API paths.

@github-actions
Copy link
Contributor

Add proper schemas for the request and response bodies in the OpenAPI spec instead of using vague type: object. Refer to the API docs for details on the event and other object structures.

@github-actions
Copy link
Contributor

The request body schema for POST /events is defined as type: object, but per the API docs, it should accept a JSON array of event objects. Update the schema accordingly.

@github-actions
Copy link
Contributor

Update the test to use os.ReadFile instead of the deprecated ioutil.ReadFile.

@github-actions
Copy link
Contributor

The PR description and commits reference adding contract tests in tests/contract/openapi_spec_test.go, but this file is not present in the changed files or repository. Please add the contract test implementation, updated to use os.ReadFile as suggested, to validate the OpenAPI spec is loadable.

@github-actions
Copy link
Contributor

The /health endpoint is currently under global authentication due to the spec's global security setting. Override it with security: [] to allow unauthenticated access, matching the API docs and server code.

@github-actions
Copy link
Contributor

@Pertempto The OpenAPI paths must start with /api/v1 to match the actual server route group (e.g., change /health to /api/v1/health). Without this, the validate script will test incorrect URLs.

@github-actions
Copy link
Contributor

Update .redocly.lint-ignore.yaml for /api/v1/health instead of /health.

@github-actions
Copy link
Contributor

Add a servers block to the OpenAPI spec to define the base URL for the API endpoints, e.g.:

servers:
  - url: http://localhost:8080

@github-actions
Copy link
Contributor

Update .redocly.lint-ignore.yaml to reflect the new path /api/v1/health instead of /health. Change the path in the ignore rule to #/paths/~1api~1v1~1health/get/responses.

@github-actions
Copy link
Contributor

Change the 401 response for POST /api/v1/user/exchangeToken to 404 with description "Token not found or expired" to match the intended behavior for invalid tokens.

@github-actions
Copy link
Contributor

@Pertempto Update the code in src/handlers/user.go to return http.StatusNotFound (404) instead of http.StatusUnauthorized (401) for invalid setup tokens, and change the error message to "Token not found or expired". This aligns with the RESTful approach for resource-like tokens.

@github-actions
Copy link
Contributor

Add a servers block to specs/openapi.yaml to define the base URL as http://localhost:8080. Without this, the base URL is not specified explicitly.

@github-actions
Copy link
Contributor

Add global security to specs/openapi.yaml:

security:
  - ApiKeyAuth: []

Then ensure unauthenticated endpoints like /api/v1/health and /api/v1/user/exchangeToken override with security: []. Currently, no security is applied to protected endpoints.

@github-actions
Copy link
Contributor

In specs/openapi.yaml, change the response for POST /api/v1/user/exchangeToken from 401 to 404 with "Token not found or expired" to align with the intended behavior when the token is invalid.

@github-actions
Copy link
Contributor

Update src/handlers/user.go in the PostSetupExchangeToken function: return http.StatusNotFound instead of http.StatusUnauthorized and change the error message to "Token not found or expired" for invalid setup tokens.

@github-actions
Copy link
Contributor

Update .redocly.lint-ignore.yaml to ignore the path #/paths/~1api~1v1~1health/get/responses instead of #/paths/~1health/get/responses since the endpoint path is /api/v1/health.

@github-actions
Copy link
Contributor

@Pertempto The path changes to /api/v1/* in the spec have been addressed, but confirm that all endpoints match the current router group in main.go with /api/v1 prefix.

@github-actions
Copy link
Contributor

The PR description and commits reference adding contract tests in tests/contract/openapi_spec_test.go, but this file is not present in the changed files or repository. Please add the contract test implementation, updated to use os.ReadFile as suggested, to validate the OpenAPI spec is loadable.

The contract test has been replaced with bash scripts (validate_openapi.sh and verify_openapi.sh), which perform OpenAPI linting with Redocly and API validation with Schemathesis against a running server. This aligns with the updated approach in the spec.

No tests/contract/openapi_spec_test.go file is needed, as the bash scripts fulfill the contract testing intent.

@github-actions
Copy link
Contributor

Update .redocly.lint-ignore.yaml to reflect the renamed endpoint paths. Change the ignore path from #/paths/~1health/get/responses to #/paths/~1api~1v1~1health/get/responses to match the /api/v1/health endpoint.

@github-actions
Copy link
Contributor

In specs/openapi.yaml, change the error response for invalid setup tokens in the POST /api/v1/user/exchangeToken operation from 401 Unauthorized to 404 Not Found with description "Token not found or expired".

@github-actions
Copy link
Contributor

In src/handlers/user.go in the PostSetupExchangeToken function, update the error response for invalid tokens to return http.StatusNotFound (404) instead of http.StatusUnauthorized (401), and change the error message to "Token not found or expired" for better REST semantics.

@github-actions
Copy link
Contributor

Add a servers block to the root level of specs/openapi.yaml with the base URL:

servers:
  - url: http://localhost:8080

The paths are prefixed with /api/v1, and CI uses http://localhost:8080, so this ensures client SDKs point to the correct base URL.

@github-actions
Copy link
Contributor

Add global security to the root level of specs/openapi.yaml:

security:
  - ApiKeyAuth: []

This applies authentication to all protected endpoints. The /api/v1/health and /api/v1/user/exchangeToken operations already specify security: [] to override it for unauthenticated access, which is good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: typescript SDK

2 participants